Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Alerts to product features. #13357

Merged
merged 3 commits into from
Jan 18, 2017
Merged

Conversation

jeff-phillips-18
Copy link
Member

@jeff-phillips-18 jeff-phillips-18 commented Jan 4, 2017

This PR adds Alerts to the product features. The UI for the the Alerts is coming in the manageiq-ui-classic repo.

@serenamarie125 @dclarizio @moolitayer

@dclarizio
Copy link

@jeff-phillips-18 we may have some conflict or confusion around the existing alerts product features around line 1902 in that file. Is this new UI code going to replace the policy alerts area?

@h-kataria can you take a look to see if there will be an conflicts or overlap with the existing features?

@jeff-phillips-18
Copy link
Member Author

Thanks @dclarizio. The existing Alerts is under Policy. Should I put the new Alerts section under Monitor then? The navigation model will be Monitor -> Alerts -> Overview... Hopefully differently nested names don't conflict.

@dclarizio
Copy link

@jeff-phillips-18 I would think it would be better under Monitor, as we try to make the features tree follow the UI menus as much as possible. The nesting doesn't avoid conflict, each feature must have a unique ID.

@jeff-phillips-18
Copy link
Member Author

@dclarizio I've updated the identifiers and put the new Alerts sections under the parent Monitor section.

@chessbyte
Copy link
Member

chessbyte commented Jan 5, 2017

@dclarizio Should this file be moved to the https://github.com/ManageIQ/manageiq-ui-classic repo?

@h-kataria
Copy link
Contributor

@dclarizio looks good, identifiers in this PR have uniq ids and do not have any conflict with existing feature ids

@dclarizio
Copy link

@chessbyte

@dclarizio Should this file be moved to the https://github.com/ManageIQ/manageiq-ui-classic repo?

I don't think so, since it controls access across all UIs and the API.

@dclarizio
Copy link

@jeff-phillips-18 Can you post a screenshot of where this appears in the Product Features tree on the Role screen in Configuration? Asking, because it should match where it appears in the navigation tabs, as we try to mimic that order to make it more usable (I don't know where Monitoring will be placed).
Thx, Dan

@jeff-phillips-18
Copy link
Member Author

@dclarizio This mimics the plan for the navigation:

image

@serenamarie125
Copy link

@jeff-phillips-18 +1 that Alerts should be under Monitor and Monitor should be the last item in primary navigation

@serenamarie125
Copy link

Underneath Alerts should be:
Overview
All Alerts
Most Recent Alerts
Top Triggers ( although this may not have been implemented yet )

@jeff-phillips-18 @dclarizio ^^

@jeff-phillips-18
Copy link
Member Author

jeff-phillips-18 commented Jan 5, 2017

@dclarizio @serenamarie125 Update:

image

We can add Top Triggers once we are closer to doing the UI work for it.

@miq-bot
Copy link
Member

miq-bot commented Jan 5, 2017

Checked commits jeff-phillips-18/manageiq@a385530~...0ed82be with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
0 files checked, 0 offenses detected
Everything looks good. 🏆

@dclarizio
Copy link

@jeff-phillips-18 looks good.
@serenamarie125 it looks like there are not CRUD actions at this time, but that's fine as new functions can be easily added in the future as needed.
@chessbyte this looks ready to merge to me.

@dclarizio
Copy link

@miq-bot assign @chessbyte

@miq-bot miq-bot assigned chessbyte and unassigned dclarizio Jan 5, 2017
@moolitayer moolitayer mentioned this pull request Jan 15, 2017
@chessbyte chessbyte merged commit b70d2ae into ManageIQ:master Jan 18, 2017
@chessbyte chessbyte added this to the Sprint 53 Ending Jan 30, 2017 milestone Jan 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants